New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gravsearch): Optimise Gravsearch queries using topological sort (DSP-1327) #1813
Conversation
…ated by topologicalOrder
…tatements first. - Fix DSP-1361.
@SepidehAlassi In {
?thing anything:hasRichtext ?richtext .
FILTER knora-api:matchText(?richtext, "test")
?thing anything:hasInteger ?int .
?int knora-api:intValueAsInt 1 .
}
UNION
{
?thing anything:hasText ?text .
FILTER knora-api:matchText(?text, "test")
?thing anything:hasInteger ?int .
?int knora-api:intValueAsInt 3 .
} After topological sort, it looks like this: {
?thing anything:hasRichtext ?richtext .
?int knora-api:intValueAsInt 1 .
?thing anything:hasInteger ?int .
FILTER(knora-api:matchText(?richtext, "test"))
} UNION {
?thing anything:hasText ?text .
?int knora-api:intValueAsInt 3 .
?thing anything:hasInteger ?int .
FILTER(knora-api:matchText(?text, "test"))
} I'm wondering if there's a way to make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for all your work on this!
Thanks for doing most of it and for reviewing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just remembered we have not added any documentation about topological sorting and its feature toggle. Let's add that before merging this.
BTW,
well, topological sorting works with the degree of nodes; here both |
… to Kahn's algorithm
…iss/knora-api into wip/DSP-1327-gravsearch
@benjamingeer So I adjusted the code to get all permutations of the topological order according to Kahn's algorithm. For the following query {
?thing anything:hasRichtext ?richtext .
FILTER knora-api:matchText(?richtext, "test")
?thing anything:hasInteger ?int .
?int knora-api:intValueAsInt 1 .
}
UNION
{
?thing anything:hasText ?text .
FILTER knora-api:matchText(?text, "test")
?thing anything:hasInteger ?int .
?int knora-api:intValueAsInt 3 .
} After the topological sort, now it looks like this: {
?int knora-api:intValueAsInt 1 .
?thing anything:hasRichtext ?richtext .
?thing anything:hasInteger ?int .
FILTER(knora-api:matchText(?richtext, "test"))
} UNION {
?int knora-api:intValueAsInt 3 .
?thing anything:hasText ?text .
?thing anything:hasInteger ?int .
FILTER(knora-api:matchText(?text, "test"))
} As you can see with the corrected version of the code, For the first block of the union, there are two valid topological sorting permutations: O1=(?thing, ?richtext, ?int, 1) and O2=(?thing, ?int, ?richtext, 1). As can be seen above, O2 is chosen to sort the statements. However, I believe maybe we should define logic to prefer O1 instead to make the query even faster. What do you think? |
Excellent!
Sounds good to me, how do we do that? |
I wrote down the idea in DSP-1327. This can be implemented in a separate PR. |
…h topological sorting
@benjamingeer I added the documentation, can you please proof-read it and add any missing points? |
This looks good to me, I proofread the docs. If you approve the PR we can merge it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjamingeer thank you.
resolves DSP-1327
resolves DSP-1361